-
-
Notifications
You must be signed in to change notification settings - Fork 2
feature(plugins): add dashless and always-trust from Lumi #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
amsryq
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR.
I might not accept Dashless plugin due to its patching approach (refer to the comments). If you don't feel like reworking on it, you can exclude this plugin from this PR and only leave AlwaysTrust
Please test your PR and run the linter if possible
src/plugins/always-trust/index.ts
Outdated
| @@ -0,0 +1,19 @@ | |||
| import { Dev } from "@data/constants"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be Devs?
src/plugins/always-trust/index.ts
Outdated
| export default definePlugin({ | ||
| name: "Always Trust", | ||
| description: "Prevents Discord's trust website confirmations", | ||
| authors: [Dev.cocobo1], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cocobo1 is not defined in in Devs
src/plugins/always-trust/index.ts
Outdated
| import { byStoreName } from "@metro/common/stores"; | ||
|
|
||
| export default definePlugin({ | ||
| name: "Always Trust", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wintry's plugin name must be PascalCase
src/plugins/dashless/index.ts
Outdated
| { | ||
| "name": "Awesomegamergame", | ||
| "id": 504401951623086081n | ||
| }], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This person is not a direct contributor to Wintry. Please credit them only at the codebase level.
If you're submitting this plugin on their behalf (they are the original author and intend to contribute to Wintry), include their identity in the Devs constant. Also, remove your name if you had no part in its development.
src/plugins/dashless/index.ts
Outdated
| unpatchRender = after(View, "render", (_, res) => { | ||
| return traverseAndModify(res); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no better approach than patching the View? This introduces performance overhead (especially for a simple plugin) and should be avoided
src/plugins/dashless/index.ts
Outdated
| return node; | ||
| }; | ||
|
|
||
| let unpatchRender: () => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wintry provides a scoped patcher. Use it instead to ensure automatic unpatching during the plugin cleanup cycle:
import { patcher } from "#plugin-context";
src/plugins/dashless/index.ts
Outdated
|
|
||
| const { View } = ReactNative; | ||
|
|
||
| const traverseAndModify = (node: any): any => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this replace every string node instead of only "changing dashes in text channel names to spaces"?
|
oh yeah i just realised I need to change a few things from lumi to make it work 😅. I kinda just rushed this pr and slammed the plugins into it, ill review your changes and fix them later today |
This adds some plugins from lumi to wintry